Skip to content

url: add fast path to getPathFromURL decoder #60749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

url: add fast path to getPathFromURL decoder #60749

wants to merge 2 commits into from

Conversation

@gurgunday
Copy link
Member

@gurgunday gurgunday commented Nov 16, 2025

branch:

node % ./node benchmark/run.js --filter whatwg-url-to-and-from-path url
url/whatwg-url-to-and-from-path.js
url/whatwg-url-to-and-from-path.js n=5000000 input="file:///dev/null" method="fileURLToPath": 3,853,044.868707496
url/whatwg-url-to-and-from-path.js n=5000000 input="file:///dev/null?key=param&bool" method="fileURLToPath": 3,490,012.0935340663
url/whatwg-url-to-and-from-path.js n=5000000 input="file:///dev/null?key=param&bool#hash" method="fileURLToPath": 3,167,181.914377771

main:

node % ./node benchmark/run.js --filter whatwg-url-to-and-from-path url
url/whatwg-url-to-and-from-path.js n=5000000 input="file:///dev/null" method="fileURLToPath": 2,034,421.4266934511
url/whatwg-url-to-and-from-path.js n=5000000 input="file:///dev/null?key=param&bool" method="fileURLToPath": 2,062,544.6025270298
url/whatwg-url-to-and-from-path.js n=5000000 input="file:///dev/null?key=param&bool#hash" method="fileURLToPath": 2,022,311.8969784945

The majority of file paths won't need decoding, so this PR avoids calling decodeURIComponent when the path is pure ASCII with no encodings

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 16, 2025

Review requested:

  • @nodejs/url

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Nov 16, 2025
@gurgunday gurgunday added the performance Issues and PRs related to the performance of Node.js. label Nov 16, 2025
@codecov
Copy link

codecov bot commented Nov 16, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.55%. Comparing base (2856475) to head (d756a3c).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/url.js 50.00% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #60749      +/-   ##
==========================================
+ Coverage   88.54%   88.55%   +0.01%     
==========================================
  Files         703      703              
  Lines      208222   208256      +34     
  Branches    40140    40152      +12     
==========================================
+ Hits       184360   184420      +60     
+ Misses      15882    15836      -46     
- Partials     7980     8000      +20
Files with missing lines Coverage Δ
lib/internal/url.js 93.59% <50.00%> (+0.24%) ⬆️

... and 41 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

aduh95
aduh95 reviewed Nov 16, 2025
View reviewed changes
@gurgunday
Copy link
Member Author

gurgunday commented Nov 25, 2025

Can we run the benchmark and the CI if this looks good? I can't do it unfortunately..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95 aduh95 aduh95 left review comments

@anonrig anonrig anonrig approved these changes

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gurgunday @nodejs-github-bot @anonrig @aduh95